Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure the server does not crash when trying to change bags in the inventory while cloaked #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oversword
Copy link

@oversword oversword commented Jul 19, 2024

When using the cloaking mod, and cloaking yourself using /cloak to cloak yourself, this error is thrown when you try to look in one of your bags:

2024-07-18 18:18:18: ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod 'unified_inventory' in callback on_playerReceiveFields(): /home/billys/mt/bin/../mods/unified_inventory/bags.lua:46: attempt to index a nil value
2024-07-18 18:18:18: ERROR[Main]: stack traceback:
2024-07-18 18:18:18: ERROR[Main]:     /home/billys/mt/bin/../mods/unified_inventory/bags.lua:46: in function 'get_player_bag_stack'
2024-07-18 18:18:18: ERROR[Main]:     /home/billys/mt/bin/../mods/unified_inventory/bags.lua:107: in function 'func'

This MR ensures the server will not crash if the bag inventory cannot be found

@oversword oversword closed this Jul 19, 2024
@oversword oversword reopened this Jul 19, 2024
@oversword
Copy link
Author

@SmallJoker minor bugfix that does not change behaviour, just adds more defensive code

@SmallJoker
Copy link
Member

Reproduced.

WARNING[Main]: GUIInventoryList::draw(): The inventory location "detached:singleplayer_bags" doesn't exist
WARNING[Main]: GUIInventoryList::draw(): The inventory location "detached:singleplayer_bags" doesn't exist
WARNING[Main]: GUIInventoryList::draw(): The inventory location "detached:singleplayer_bags" doesn't exist
WARNING[Main]: GUIInventoryList::draw(): The inventory location "detached:singleplayer_bags" doesn't exist
ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod 'unified_inventory' in callback on_playerReceiveFields(): MT/mods/unified_inventory/bags.lua:46: attempt to index a nil value
ERROR[Main]: stack traceback:
ERROR[Main]: 	MT/mods/unified_inventory/bags.lua:46: in function 'get_player_bag_stack'
ERROR[Main]: 	MT/mods/unified_inventory/bags.lua:122: in function 'func'
ERROR[Main]: 	MT/builtin/profiler/instrumentation.lua:124: in function <MT/builtin/profiler/instrumentation.lua:117>
ERROR[Main]: 	MT/builtin/common/register.lua:26: in function <MT/builtin/common/register.lua:12>

@SmallJoker
Copy link
Member

SmallJoker commented Jul 19, 2024

Why does minetest.get_inventory return nil? I cannot find any overwrites in the cloak mod, neither does it remove detached inventories.
It seems to me that this is not the correct solution. The inventory must be present as long the player is recognized as "online". This should be solved by preventing the inventory callback at the root by checking the validity of player - unrelated to the detached inventories.

EDIT: Whatever this mod does, it also affects 3d_armor when trying to move items:

WARNING[Server]: set_textures called on offline player
WARNING[Server]: set_textures called on offline player
WARNING[Server]: set_textures called on offline player

@oversword
Copy link
Author

@SmallJoker
As far as I know cloaking basically simulates being offline somehow, I don't think this defensive code can hurt right? If it can return nil it should account for that.

Though I would like to find a better solution later on to make using bags possible, but I think that's a minetest problem - can't access a player's inventory when offline (which is annoying because you can't find accounts with suspicious items who rarely log on)

@SmallJoker
Copy link
Member

@oversword cloak fakes the player leave by calling minetest.registered_on_leaveplayers. Hence unified_inventory removes the detached inventory by itself. I think the proper fix is to check against unified_inventory.players[player_name]. It seems that we yet need to set this to nil in callbacks.lua L260.

@oversword
Copy link
Author

@SmallJoker I took another look, I'm not sure unified_inventory.players does anything... it's never referenced as far as I can tell.
However what you said is helpful, I hadn't looked into the cloaking mod before, I have a version with bags working while cloaked that depends on this:
luk3yx/minetest-cloaking#10

Which basically lets you tell when someone is cloaking or uncloaking during the leaveplayer and joinplayer callbacks, this can be used to prevent detatching the inventories

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants